-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC:Use of "Yields" for documentation of DataFrame.iteritems() #27876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
changed `Returns` to `Yields` closes #27860
@datapythonista @WillAyd : The fix looks good, but the documentation checks seem to be failing. |
See the also the discussion on the issue: #27860. Someone will need to understand why the docstring validation sees iteritems and items differently |
Ah, it is because https://github.com/pandas-dev/pandas/blob/master/scripts/validate_docstrings.py#L529-L562 That's of course a bit brittle. @datapythonista I suppose the easiest here is to ignore that rule? |
Since we're Python 3 only now, my preference is to use |
changed `return self.items`` --> `yield from self.items()` for better readability and fix doc string failures
pandas/core/frame.py
Outdated
@@ -825,9 +825,9 @@ def items(self): | |||
for i, k in enumerate(self.columns): | |||
yield k, self._ixs(i, axis=1) | |||
|
|||
@Appender(_shared_docs["items"] % "Returns\n -------") | |||
@Appender(_shared_docs["items"] % "Yields\n -------") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sameshl you can actually remove this % Yields
now, and directly edit this in the docstring above. This was only done so that items
and iteritems
could have a different title in the docstring (returns vs yields)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh. Nice suggestion @jorisvandenbossche. Will make the required change.
thanks @datapythonista that indeed seems a good idea |
removed placeholder in shared docstring of iteritems and directly changed the main docstring so both of them read `Yields` rather than `Return`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, thanks @sameshl
I think it'd probably be cleaner if def items
had the docstring directly (just a normal docstring), and def iteritems
uses @Appender(DataFrame.items)
(I think that's what we use in these cases).
Didn't check in detail if that makes sense for sure, and happy to do that in a follow up PR.
Thanks @sameshl ! |
Glad to help! @jorisvandenbossche |
changed
Returns
toYields
closes #27860